-
Notifications
You must be signed in to change notification settings - Fork 16
Detect soundtrack based on the release title #139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect soundtrack based on the release title #139
Conversation
Sorry, I see the test case failed now. Must have been a last minute change I did, since titles starting and ending with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that looks promising. I'm not at home right now, so it might take me until next week to review your contributions properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the general implementation and the large collection of test cases, well done!
My main worry is the additional complexity (and processing time) that we get with each new pattern since these are executed for every release (among them many non-soundtracks).
Currently we try all patterns without exiting early once we have a match. That mostly makes sense for the previously existing patterns where something can be detected as both an EP and Live, for example.
But for the new soundtrack patterns we no longer have to check the remaining ones once we have a match.
So I see a potential to optimize this function by grouping patterns by release type, but you don't have to do that in this PR.
Especially having specific rules for too many languages could be problematic if we are not careful.
I will accept these for now since you have already implemented them (and they are useful for my own selfish needs 😇), but probably won't accept language-specific rules in the future.
My plan is that such rules can be customized client-side later on, as part of the implementation of #130, which will allow us to have disabled rules.
Might not be what you had in mind, but I added a check in the loop now that skips additional pattern checks if the pattern is for a release type which the release already has been assigned, i.e. if it has already been assigned as a soundtrack no reason to match it against additional soundtrack patterns. |
Closes: #105
Implements auto-detection of soundtrack releases based the title based on a wide range of common title patterns. Supports both common title formats in English (which often are used for releases in other languages as well), German, Swedish, and Norwegian.
I've added test cases for a wide variation of soundtracks.
In contrast to the existing patterns for detecting release group types from the title, the pattern does not always capture the type and as such I had to come up with a way of associating the pattern to a type. I think this can probably be useful for when detecting more release group types from titles in the future.